Repository navigation
fix(browser): create instance before draining cmd queue; isolate throwing commands - #383
Open
jrosendahl-opt wants to merge 1 commit into
Open
jrosendahl-opt wants to merge 1 commit into
jrosendahl-opt wants to merge 1 commit into
Conversation
…wing commands The script-tag bundle drained window.optable.cmd before creating the instance from instance_config, so a command queued before the async bundle ran saw optable.instance === undefined and threw. browser/commands.ts had no try/catch, so that throw aborted the bundle's top level: no instance, no utils, and cmd stayed a plain array so later pushes never ran. Create the instance first and drain the queue with the existing lib/addons/commands OptableCommands, which catches per command. Remove the now-unused browser/commands.ts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
With automatic initialization (
instance_config, README "Option 1"), the CDN bundle drainedwindow.optable.cmdbefore creatingwindow.optable.instance. If the async bundle ran afterDOMContentLoaded(common), the README's queuedoptable.instance.identify(...)threwTypeError: Cannot read properties of undefined.Because
browser/commands.tshad no try/catch, that throw aborted the bundle's top-level setup:instancewas never created (no passport/pageview),utilswas never set, andcmdstayed a plain array so later pushes never ran. The same happened in "Option 2" for any queued command that throws.Found while auditing the README against the code.
What Changed
browser/sdk.ts: setutilsand create the instance before draining the queue.browser/sdk.ts: drain withlib/addons/commands(OptableCommands), which catches and logs per command, so the "a throwing command cannot break init" guarantee inlib/addons/commands.mdnow holds for the CDN bundle too.browser/commands.ts(no longer used and does not appear to be used in bundles).browser/sdk.test.ts: new tests that load the bundle entry with queued commands.